Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat element relation view #128

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Feat element relation view #128

wants to merge 13 commits into from

Conversation

huyenngn
Copy link
Member

No description provided.

@huyenngn huyenngn marked this pull request as ready for review August 1, 2024 12:30
Because of elkjs limitations edge labels cannot be displayed properly at this point without sacrificing the desired layout.
Edge labels will be omitted completely as to not create confusion with them showing up on the top left corner.
They should be added back when the issues with the edge labels got fixed.
@huyenngn huyenngn force-pushed the feat-element-relation-view branch from 563d5e5 to 88e15a1 Compare August 2, 2024 08:29
@huyenngn huyenngn requested a review from micha91 August 5, 2024 08:11
Copy link
Member

@ewuerger ewuerger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job. I am not sure about the naming of this View. We bring ExchangeItems and Classes with ExchangeItemElements into a relation. But we also reuse the ClassTreeView. Maybe ExchangeItemClassTreeView? I am open for ideas.

Never the less, we need documentation here:

  • Add a exchange_item_class_tree_view.md in docs (add it to mkdocs.yml)
    • Hint to the problem of routing edges (ExchangeItemElements) which could be solved when Libavoid comes publically available
  • Describe collection, parameters and give examples (reuse the test case)
  • Add the needed SVGs to the docs/gen_images.py

capellambse_context_diagrams/__init__.py Outdated Show resolved Hide resolved
capellambse_context_diagrams/__init__.py Outdated Show resolved Hide resolved
capellambse_context_diagrams/context.py Outdated Show resolved Hide resolved
capellambse_context_diagrams/context.py Outdated Show resolved Hide resolved
tests/test_element_relation_views.py Outdated Show resolved Hide resolved
tests/test_element_relation_views.py Outdated Show resolved Hide resolved
@huyenngn huyenngn force-pushed the feat-element-relation-view branch 3 times, most recently from 5c4fab9 to 0338310 Compare August 13, 2024 08:32
@huyenngn huyenngn requested a review from ewuerger August 13, 2024 08:34
@huyenngn huyenngn force-pushed the feat-element-relation-view branch from 0338310 to 84ebc9a Compare August 13, 2024 09:29
Copy link
Member

@ewuerger ewuerger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just rename it back to exchange_item_relation_view.
LGTM.

@huyenngn huyenngn requested a review from ewuerger August 21, 2024 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants